Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove many deprecated pylint options #329

Merged
merged 14 commits into from
Feb 18, 2024
Merged

Conversation

hmaarrfk
Copy link
Contributor

These were mostly useful for the python2->python3 transition

pylint-dev/pylint#4942

Relevant issues/PRs:

@leouieda
Copy link
Member

@hmaarrfk thanks for this! Let me know when this is ready for a review. There are quite a few changes here but I’m assuming they’re related to making pylint pass, right?

@hmaarrfk hmaarrfk force-pushed the lint_pylint branch 4 times, most recently from ed8ad60 to 9ba00a9 Compare October 22, 2022 14:21
@hmaarrfk
Copy link
Contributor Author

@leouieda i think this is ready. it is likely best to review each commit one at a time.

They are each organized in a thematic manner and it makes it more clear what the improvement is.

There are some (such as the timeout) that may be worth more discussion.

@leouieda
Copy link
Member

@hmaarrfk sorry for the very long delay in this. Could you please elaborate on why adding the default timeout of 5? Is that a pylint recommendation?

@hmaarrfk
Copy link
Contributor Author

hmaarrfk commented Oct 1, 2023

Not specifying the timeout really causes requests to hang.... it might have been a linting recommendation, but I've experienced this alot in our "web connected" application.

If you want to support offline, you must specify a timeout mostly everywhere.

@hmaarrfk
Copy link
Contributor Author

hmaarrfk commented Oct 1, 2023

I rebased and I think thingsare ok. let me know if iyou want anything else.

@leouieda
Copy link
Member

Just waiting on #373 to make sure everything is running properly here before merging.

@leouieda leouieda merged commit 1204d76 into fatiando:main Feb 18, 2024
18 checks passed
@leouieda
Copy link
Member

Finally merging this! Thanks for all your hard work @hmaarrfk 🫶

@hmaarrfk
Copy link
Contributor Author

no problem!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants